feat: add camera presets for 3D snapshots and validation#2187
feat: add camera presets for 3D snapshots and validation#2187rushabhcodes wants to merge 3 commits intotscircuit:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a camera-preset feature to the tsci snapshot CLI path, enabling consistent 3D snapshot viewpoints via named presets, and introduces a dedicated test suite plus committed snapshot artifacts.
Changes:
- Added
lib/shared/camera-presets.tsto define named camera presets and an API to apply/validate them. - Updated
snapshotProjectto accept an optionalcameraPresetthat implies--3dand modifies the default camera before rendering. - Extended the CLI
snapshotcommand with--camera-presetvalidation and added camera-preset CLI tests + committed PNG artifacts.
Reviewed changes
Copilot reviewed 4 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cli/snapshot/register.ts |
Adds --camera-preset option, validates preset value, and forwards it to snapshotProject. |
lib/shared/snapshot-project.ts |
Enables 3D output when a camera preset is provided and applies the preset to the computed camera. |
lib/shared/camera-presets.ts |
Introduces preset definitions and preset-application/validation helper. |
tests/cli/snapshot/snapshot-camera-preset.test.ts |
New CLI tests for preset success/failure paths and basic output validation. |
tests/cli/snapshot/__snapshots__/3d-top-down.snap.png |
Adds committed 3D snapshot artifact for top-down. |
tests/cli/snapshot/__snapshots__/3d-top-center-angled.snap.png |
Adds committed 3D snapshot artifact for top-center-angled. |
tests/cli/snapshot/__snapshots__/3d-front.snap.png |
Adds committed 3D snapshot artifact for front. |
tests/cli/snapshot/__snapshots__/3d-left-sideview.snap.png |
Adds committed 3D snapshot artifact for left-sideview. |
tests/cli/snapshot/__snapshots__/3d-right-sideview.snap.png |
Adds committed 3D snapshot artifact for right-sideview. |
tests/cli/snapshot/__snapshots__/3d-top-left.snap.png |
Adds committed 3D snapshot artifact for top-left. |
tests/cli/snapshot/__snapshots__/3d-top-left-corner.snap.png |
Adds committed 3D snapshot artifact for top-left-corner. |
tests/cli/snapshot/__snapshots__/3d-top-right.snap.png |
Adds committed 3D snapshot artifact for top-right. |
tests/cli/snapshot/__snapshots__/3d-top-right-corner.snap.png |
Adds committed 3D snapshot artifact for top-right-corner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Save the generated snapshot with a preset-specific name for inspection | ||
| fs.mkdirSync(SAVED_SNAPSHOTS_DIR, { recursive: true }) | ||
| fs.copyFileSync( | ||
| join(snapshotDir, "test.board-3d.snap.png"), | ||
| join(SAVED_SNAPSHOTS_DIR, `3d-${preset}.snap.png`), | ||
| ) |
There was a problem hiding this comment.
These tests write generated PNGs into tests/cli/snapshot/__snapshots__ in the repo (fs.copyFileSync(...)). That makes the test suite non-hermetic and can dirty the working tree / cause flakiness in environments that enforce a clean checkout. Consider asserting against committed snapshots (or keeping artifacts only in the temp fixture directory / behind an explicit env flag) instead of copying into the repo during normal test runs.
| if (!(preset in CAMERA_PRESETS)) { | ||
| throw new Error( | ||
| `Unknown camera preset "${preset}". Valid presets: ${CAMERA_PRESET_NAMES.join(", ")}`, | ||
| ) | ||
| } | ||
| return CAMERA_PRESETS[preset as CameraPreset](cam) |
There was a problem hiding this comment.
applyCameraPreset uses preset in CAMERA_PRESETS to validate. The in operator also returns true for properties on the prototype chain (e.g. "toString"), which would bypass the error and then attempt to call a non-preset value. Use an own-property check (e.g. Object.hasOwn / hasOwnProperty) to ensure unknown strings reliably throw.
seveibar
left a comment
seveibar
left a comment
There was a problem hiding this comment.
Good but top down is wrong
seveibar
left a comment
seveibar
left a comment
There was a problem hiding this comment.
Also please add top-down-ortho
|
Can be added in next pr |
This pull request adds support for camera angle presets when generating 3D snapshots via the CLI, allowing users to select different viewpoints for rendered images. It introduces a new shared module for camera presets, updates the snapshot logic to apply these presets, and includes comprehensive tests to ensure correct behavior and error handling.
https://linear.app/tscircuit/issue/TSC-397/camera-presets-for-tsci-snapshot-3d-snapshots
Camera Preset Feature:
lib/shared/camera-presets.tsmodule that defines multiple named camera angle presets, a type-safe API for applying them, and validation utilities. This enables consistent, reusable camera positioning for 3D renders.snapshotcommand now accepts a--camera-preset <preset>option, validates the preset, and applies it to 3D renders. Using this option automatically enables 3D snapshot generation. [1] [2] [3] [4]Snapshot Logic Updates:
snapshotProjectfunction inlib/shared/snapshot-project.tsis updated to accept an optionalcameraPresetparameter, which, if provided, overrides the default camera angle for 3D renders and ensures 3D output is generated. [1] [2] [3] [4]Testing:
tests/cli/snapshot/snapshot-camera-preset.test.ts) that verifies all camera presets generate valid 3D snapshots, checks for correct error handling with invalid presets, ensures--camera-presetimplies 3D output, and confirms that different presets produce distinct images.